Skip to content

feat(mcp-oauth): support static clientId for servers without DCR#379

Open
proyectoauraorg wants to merge 5 commits into
Zoo-Code-Org:mainfrom
proyectoauraorg:feat/mcp-oauth-static-clientid
Open

feat(mcp-oauth): support static clientId for servers without DCR#379
proyectoauraorg wants to merge 5 commits into
Zoo-Code-Org:mainfrom
proyectoauraorg:feat/mcp-oauth-static-clientid

Conversation

@proyectoauraorg
Copy link
Copy Markdown
Contributor

@proyectoauraorg proyectoauraorg commented May 28, 2026

Related GitHub Issue

Aligned with VS Code 1.122 feature #257415 — MCP OAuth with custom clientId.

Description

Adds optional oauth.clientId field to MCP server configuration. When provided, the OAuth provider uses this clientId directly instead of performing Dynamic Client Registration (DCR). This enables connections to OAuth-protected MCP servers that don't support RFC 7591 DCR.

Use case: Many corporate and self-hosted MCP servers require a pre-registered clientId but don't support DCR. Currently, Zoo Code cannot connect to these servers via OAuth.

Changes:

  • BaseConfigSchema: add oauth.clientId optional field
  • McpOAuthClientProvider: accept clientId in create() options, use it in registerClientIfNeeded() to skip DCR
  • McpHub: pass oauth.clientId from config to the provider
  • Tests: 2 new tests covering static clientId and precedence over cached DCR data

Example configuration in .roo/mcp.json:

{
  "mcpServers": {
    "corporate-server": {
      "type": "streamable-http",
      "url": "https://mcp.corporate.example.com",
      "oauth": {
        "clientId": "my-app-id-12345"
      }
    }
  }
}

Test Procedure

Unit tests (104 passed):

cd src && npx vitest run services/mcp/__tests__/McpOAuthClientProvider.spec.ts services/mcp/__tests__/McpHub.spec.ts
  • McpOAuthClientProvider.spec.ts — 44 tests (42 existing + 2 new for static clientId)
  • McpHub.spec.ts — 60 tests (all existing, schema change verified)

New tests:

  1. should use static clientId instead of performing DCR — verifies clientId is used directly
  2. should use static clientId even when cached data exists — verifies static takes precedence over cached DCR

Pre-Submission Checklist

  • Issue Linked: Aligned with VS Code 1.122 feature.
  • Scope: Focused on static clientId support only.
  • Self-Review: Thorough review performed.
  • Testing: 2 new tests added, 104/104 pass.
  • Documentation Impact: No doc updates needed (self-documenting config field).
  • Contribution Guidelines: Read and agreed.

Documentation Updates

  • No documentation updates are required.

Summary by CodeRabbit

  • New Features

    • Server config now accepts an optional OAuth client ID so OAuth-backed connections can use a configured client identifier.
  • Tests

    • Added tests for static OAuth client ID behavior, including cached-credentials paths.
    • Added advanced streaming scenario tests covering delta concatenation, mixed-stream flushing, and usage token handling.
  • Chores

    • Updated a dependency to a newer UUID library version.

Review Change Stack

proyectoauraorg and others added 3 commits May 26, 2026 00:05
…provider

Merging 12 new test cases covering completePrompt, streaming resilience, and edge cases.
Dependabot bump. Compatible: project requires node>=20.20.2, uuid v14 requires node>=20.
Add optional oauth.clientId field to MCP server configuration schema.
When provided, the OAuth provider uses this clientId directly instead of
performing Dynamic Client Registration (DCR). This enables connections
to OAuth-protected MCP servers that don't support RFC 7591 DCR.

Changes:
- BaseConfigSchema: add oauth.clientId optional field
- McpOAuthClientProvider: accept clientId in create() options,
  use it in registerClientIfNeeded() to skip DCR
- McpHub: pass oauth.clientId from config to the provider
- Tests: 2 new tests covering static clientId and precedence over cache

Aligned with VS Code 1.122 feature: MCP OAuth with custom clientId.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 47a370be-0097-4ad4-a053-a25a1e780f27

📥 Commits

Reviewing files that changed from the base of the PR and between 396af15 and 250fd05.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • src/api/providers/__tests__/mimo.spec.ts
  • src/package.json
  • tmp/README.md

📝 Walkthrough

Walkthrough

Adds optional static OAuth clientId support: schema and McpHub wiring forward oauth.clientId to McpOAuthClientProvider.create(). Provider accepts clientId option and, when present, populates client info directly to bypass DCR/SecretStorage. Tests validate the static path. Also adds advanced Mimo stream tests and bumps uuid to ^14.0.0.

Changes

Static OAuth clientId Configuration

Layer / File(s) Summary
Configuration contract and hub integration
src/services/mcp/McpHub.ts
ServerConfigSchema accepts optional oauth: { clientId?: string }, and McpHub passes configInjected.oauth?.clientId to McpOAuthClientProvider.create() for streamable-http servers.
Provider static clientId support
src/services/mcp/McpOAuthClientProvider.ts
Constructor stores _staticClientId, create() accepts options: { skipDiscovery?, clientId? }, and registerClientIfNeeded() sets _clientInfo directly from the static client ID and redirect URL when provided, skipping SecretStorage/DCR.
Static clientId verification tests
src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts
Tests under static clientId support ensure provided clientId is used and takes precedence over cached client_info.

Stream tests and dependency bump

Layer / File(s) Summary
Mimo advanced streaming scenarios
src/api/providers/__tests__/mimo.spec.ts
Adds tests for concatenating multiple delta.content chunks, mixed reasoning_content and tool_calls streaming with tool_call_end on finish, omission of usage when final chunk has usage: null, and omission of zero-valued cache token fields.
Dependency bump
src/package.json
Bumps uuid dependency from ^11.1.0 to ^14.0.0.

Sequence Diagram(s)

sequenceDiagram
  participant McpHub
  participant McpOAuthClientProvider
  participant registerClientIfNeeded
  participant SecretStorage
  participant DCR
  McpHub->>McpOAuthClientProvider: create(..., { clientId })
  McpOAuthClientProvider->>McpOAuthClientProvider: store _staticClientId
  McpOAuthClientProvider->>registerClientIfNeeded: registerClientIfNeeded()
  registerClientIfNeeded->>registerClientIfNeeded: check _staticClientId
  alt static clientId present
    registerClientIfNeeded->>McpOAuthClientProvider: set _clientInfo {client_id, redirectUrl}
    registerClientIfNeeded-->>McpOAuthClientProvider: return
  else no static clientId
    registerClientIfNeeded->>SecretStorage: get cached client_info
    alt cached
      SecretStorage-->>registerClientIfNeeded: client_info
    else
      registerClientIfNeeded->>DCR: perform dynamic client registration
      DCR-->>registerClientIfNeeded: client_info
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • taltas
  • navedmerchant
  • hannesrudolph
  • edelauna

Poem

🐰 I hopped through configs, small and spry,
A clientId tucked beneath the sky,
No DCR dance, just one swift hop,
Tests hum softly, deps won't stop,
OAuth and streams — a carrot pie! 🍰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main feature: adding support for static clientId in MCP OAuth to avoid DCR.
Description check ✅ Passed The description is comprehensive, covering linked issue, implementation details, test procedure, and pre-submission checklist; all key sections are well-populated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/api/providers/__tests__/mimo.spec.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/services/mcp/McpHub.ts 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@edelauna
Copy link
Copy Markdown
Contributor

edelauna commented May 28, 2026

Wouldn't this also require a secret?

This would also require the whole normal OAuth 2.0 flow to be available when provided. This is larger than a 3 file change.

@taltas
Copy link
Copy Markdown
Contributor

taltas commented May 29, 2026

Thanks for the contribution. There is also the huge readme file that was committed like your other PRs, we should remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants